test/pytest: the raiser has to be the statement under test (#432) - #939
Conversation
…ompt#432) `raises-catches-setup`, entry 5 and the last on VACUITY_MODES.md's list of what to add next. The statement COUNT rule refused a `pytest.raises` block holding more than one top-level statement. Two shapes are ONE statement and still perform the setup inside the block, so the count saw nothing: with pytest.raises(psycopg.errors.UndefinedObject) as exc: _setup_then_run(conn) # a HELPER CALL: one statement with pytest.raises(psycopg.errors.UndefinedObject) as exc: for stmt in (setup_sql, sql_under_test): # a COMPOUND STATEMENT: one conn.execute(stmt) # statement holding two Both were measured against the shipped scan reporting `1 passed`, exit 0, zero offences, with the setup raising and the statement under test never running. The two arms that recorded them as residuals now assert the refusal, so the closure is a measurement rather than a sentence. THE FIX IS NOT A DEEPER COUNT, for the reason the entry always gave: counting recursively would also refuse a legitimate single-statement loop. It is a claim about WHICH statement raised, in two rules. NO COMPOUND STATEMENT, and all nine kinds rather than the one the inventory named. The document named the `for` spelling; `if`, `while`, `with` and `try` nest identically, and a rule catching only `for` would close an example rather than a mode. The kinds are looked up by name instead of written out, because `TryStar` and `Match` exist only on newer Pythons and a missing attribute would be a NameError at import rather than a rule that quietly does less. NO CALL TO A FUNCTION DEFINED IN THE SAME FILE, anywhere in the statement. Such a function can run any number of statements and nothing in the block says which failed. A call to an IMPORTED function, or to a METHOD, is the thing under test and stays allowed. THE RULE TURNS ON WHERE THE FUNCTION IS DEFINED, NOT ON THE STATEMENT BEING A CALL, and that is what makes the budget zero. Measured over the corpus before writing either rule: five `pytest.raises` blocks in real code, NONE touching a database -- four call `build_and_install`, imported from the module under test, and one calls a method. The scan reports no offence on any of them. MEASURED FIRST, AND IT CHANGED THE DESIGN. The entry suggests "a helper that runs exactly one statement and owns the assertion", which would be a DB helper. There are zero SQL-raising `pytest.raises` blocks in the corpus, so that helper would have had no call sites -- an instrument with nothing exercising it, which is the thing this directory refuses to build. The shape rules close the same mode against the code that exists. THE RESIDUAL IS A METHOD. A method that performs setup and then the statement is invisible to this rule, and no static rule can see inside it. Stated rather than hidden, and pinned by the arm that accepts the method shape. Prove by removal, five mutations, each by exact string with a parse assertion, and the restore verified byte-identical rather than by `git diff`: control 34 passed compound rule removed 2 failed local-def rule removed 2 failed only `for` counts as compound 1 failed only a bare call is searched 1 failed the message filter narrowed 4 failed TWO OF MY OWN ARMS COULD NOT FAIL UNTIL I MUTATED THEM. The local-def rule searches the WHOLE statement, and my first arm used a bare call only -- so narrowing the search to `Expr` changed nothing and `x = _helper()` stayed open. And the every-compound-kind arm exists because without it, reducing the kind list to `For` alone was invisible. AND THE RULES FIRED WHILE PRINTING NOTHING. The message assembly buckets offenders by substring, and the bucket's filter was the exact sentence of the COUNT rule -- so both new rules refused the run and the layer said `refuses this run: .` with an empty list. The arms reddened on a missing message while the refusal itself worked, which is a guard that cannot be told from an unfired one. The filter is now the common tail of all three phrases. RECORDED WHILE I WAS IN THERE: the comment defending the one-source-line offence phrase cited selftest 440 grepping this file and counting the copies. **Selftest 440 no longer exists** -- commandprompt#927 deleted it under the harness-independence rule. Nothing greps this source for the phrase today, so that form is now a convention rather than a guarded property; what is still load-bearing is the RUNTIME string the pytest arms match, which a split f-string would not change at all. The comment says that instead. Gate: harness_selftest 588 checks, 588 passed + 0 failed + 0 unrunnable, PASSED driver-free job 9 files, 161 passed, psycopg absent from the venv full corpus 241 passed with a cluster on pg18a Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a
…her branch commandprompt#935 closed `insert-wrote-no-rows` and moved the refused total 27 -> 28. This branch closes `raises-catches-setup` and moved it 27 -> 28 as well. The merge was CLEAN and therefore wrong: git took one 28 where the answer is 29, because the number is a property of both closures rather than of either. That is the exact failure TESTS.md's own header records about counts in this document -- "a claim whose correct value is a function of the MERGE rather than of either branch, so it collided on essentially every rebase" -- and the reason the counts are checked by arms instead of trusted. The arms named every number: refused today 28 -> 29 not refused 44 -> 43 demonstrated 48 -> 47 (section 3's sentence) TESTS.md prose 28 -> 29, and 44/43 -> 43/42 README.md 28 -> 29 I did not decide any of them; each one is what `test_the_prose_totals_match_the_counted_modes` and its siblings reported against the ids on disk. Gate on the merged tree: harness_selftest 588 checks, 588 passed + 0 failed + 0 unrunnable, PASSED driver-free job 10 files, 174 passed, psycopg absent from the venv full corpus 257 passed with a cluster on pg18a the raises scan zero offences over the corpus, all 9 compound kinds recognised Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a
jdatcmd
left a comment
There was a problem hiding this comment.
Approving at exact head 3fb80aba26a205c33aac3764375e36cb9f47e291. Everything below was measured on that head, not on the one I started reading: I composed #936's main into 4c8f7c19 myself, and the resulting tree oid is 8a89aac922a1e8ae1c10c9115a145c4ad5a5e615 — identical to your head's tree. So the composition is not a claim either of us has to trust.
Your five mutations, re-run rather than read
My first control said 28 failed, 6 passed and I nearly reported your table as wrong. It was my instrument: test_raises_sqlstate.py is not in NO_CLUSTER, and the driver-free venv makes every inner pytester run fail to collect on ModuleNotFoundError: psycopg. With the driver installed the control is 34 passed, and every row of your table reproduces:
| mutation | mine | yours |
|---|---|---|
| control | 34 passed | 34 passed |
| compound rule removed | 2 failed | 2 failed |
| local-def rule removed | 2 failed | 2 failed |
only For counts as compound |
1 failed | 1 failed |
| only a bare call is searched | 1 failed | 1 failed |
| the message filter narrowed | 4 failed | 4 failed |
Each mutation asserted its anchor present and the result parsed before running, and the file's md5 was compared back to baseline after each restore.
The count survived #936, derived rather than argued
You said the arms decided it. I ran them and printed what they derived:
refused ids (from section 2 on disk) 29
not refused 43
sum 72 == named
README.md contains "29 refused" 1 occurrence
Population, on the merged head: 17 files, 5 pytest.raises blocks, 0 offences.
Three residuals, measured
The entry's own standard is that a rule catching only for closes an example rather than a mode. By that standard the two new rules each leave a spelling open. None of the three is present in the corpus today — that is why none blocks — but the residual section names only "a method", and these are not that.
1. The loop is refused as a statement and allowed as an expression. One token from the shape you close:
REFUSED with pytest.raises(...): for stmt in (setup, under_test): conn.execute(stmt)
passes with pytest.raises(...): [conn.execute(s) for s in (setup, under_test)]
passes with pytest.raises(...): (conn.execute(setup), conn.execute(under_test))
Controls, so the probe can fail: an honest single statement and an honest single-statement loop are both not flagged.
2. The local-def rule is defeated by moving the helper one file over. Your own offending shape D, with nothing about what runs changed:
REFUSED def _setup_then_run(conn): ... (defined here)
passes from pgc_helpers import _setup_then_run
3. The same key refuses an honest helper. A local _run() that executes exactly one statement is refused, and becomes clean by being imported instead.
2 and 3 are one coin: where the function is defined is neither necessary nor sufficient for the vice. I think the design call is still right — four of the five real blocks call build_and_install, imported from the module under test, so keying on locality is what makes the budget zero against the code that exists. My ask is only that §3's residual paragraph say so, because a reader who moves a helper into pgc_helpers.py will get a green scan and no hint that the mode came with it.
Noted, not asked for
Your comment correcting the selftest-440 rationale is the right shape: the one-line phrase is now a convention and the runtime string is the guarded property. That is a comment that will still be true after the next deletion, which is more than the one it replaced managed.
…not have moved I claimed a closure I can defeat three ways, and @jdatcmd's review named the residuals. Measuring them settles the classification against me: a `for` loop over two statements REFUSED the same two as a list comprehension allowed the same two as a tuple of calls allowed a helper defined in ANOTHER file allowed an honest one-statement helper in THIS file REFUSED (a false positive) A comprehension and a tuple are EXPRESSIONS, not compound statements, so a rule about statement kinds cannot see them; `local_defs` is built from one file, so moving the helper one file over defeats it. Neither is a contrivance -- both are ordinary Python. So by this document's own convention the mode stays in section 3, the section-2 row is withdrawn, and the refused count goes back to 28 with not-refused back to 44. The arms derive it: refused=28, not_refused=44, and `raises-catches-setup` is not in the refused set. Section 5's entry now says "Done, and it NARROWS rather than closes", beside the two other entries that say the same thing. THE ARM THAT EXISTS TO CATCH THIS OVERCLAIM DID NOT CATCH IT, and that is the part worth keeping. `test_the_mode_this_layer_only_narrows_is_still_listed_as_open` required the mode to be named in section 3 -- and section 3 keeps a back-reference for every mode that MOVES, "`X` is now closed", so the id is present in section 3 in BOTH states. My first version wrote the closure into section 3, added the row to section 2, moved the count to 29, and the arm passed. Its own docstring says what it was for: "A document that quietly moved the mode to section 2 would claim a closure this scan does not make." It now also requires the mode to be named OUTSIDE a closure back-reference, and to be absent from section 2. Proven by removal, each shape of the overclaim separately: control (the document as it stands) 1 passed entry rewritten as a back-reference 1 failed row added back to section 2 1 failed both, which is what my branch did 1 failed AND MY FIRST TIGHTENING WAS WRONG IN THE SAME FAMILY. I wrote `section3.split("is now closed")[0]`, which truncates at the FIRST back-reference in the section -- today `insert-wrote-no-rows`, which sits above this entry -- so it reported "only as a back-reference" about a document that names the mode correctly. A positional test over a section holding several back-references is a test about their order. It is per-line now. Gate: harness_selftest 588 checks, 588 passed + 0 failed + 0 unrunnable, PASSED driver-free job 10 files, 183 passed, psycopg absent from the venv full corpus 266 passed with a cluster on pg18a derived counts refused 28, not refused 44, named 72 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a
|
Pushed You asked only that §3 name the three residuals. Measuring them to write them down settled the classification against me:
A comprehension and a tuple are expressions, not compound statements, so a rule about statement kinds cannot see them; The arm that exists to catch this overclaim did not catch it
It now also requires the mode to be named outside a closure back-reference and to be absent from §2. Proven by removal, each shape separately: And my first tightening was wrong in the same family. I wrote GateOn your #941 residual: you are right that greedy And your reconciliation residue is the nicer half of that exchange: printing |
…grep -c branch # Conflicts: # CHANGELOG.md
…eadable branch Part 010 conflicted, and resolving it found a latent defect of MINE that main's side fixes. Taking main's side wholesale, not as a compromise. THIS BRANCH KEPT `exit 0` IN A SOURCED PART, which is exactly the defect #934 names and #940 closed. My change to part 010 swapped the bare `echo "SKIP ..."` for `check_skip`, to route the outcome through pgc_record as @linuxhikerpm asked, and left the `exit 0` below it untouched. AND THE `check_skip` COULD NEVER HAVE RECORDED ANYTHING. `lib.sh` is sourced by part 020; part 010 runs before it, which is the same fact #940's own comment turns on when it spells 66 as a literal rather than `$PGC_EXIT_SKIPPED`. So `check_skip` in part 010 is not a call to a helper, it is a command that does not exist yet. Measured rather than argued: type check_skip, at the point part 010 runs NOT DEFINED calling it under the driver's `set -uo pipefail` bash: check_skip: command not found, rc=127, execution CONTINUES Under `-u -o pipefail` without `-e` that is not fatal, so the bail path would have printed a bash error, recorded nothing, and then exited the driver with status 0 -- the false green, with a diagnostic line in front of it. It has never fired because the squatter-port path does not trigger in a normal run, which is why no gate has ever executed those two lines. main's version is correct on all three axes: `exit 66` rather than `exit 0`, the `SKIPPED (ran no checks)` marker the runners pair with 66, and no call to a function that has not been defined yet. PART 010 IS THEREFORE AN EXCEPTION TO THE RULE THIS PR ARGUES, and it should be named as one rather than quietly left out. Every per-check outcome routes through `pgc_record` except in the one part that runs before `pgc_record` exists. There the right shape is a part-level bail, which is what #940 built. Checked rather than eyeballed, on the merged tree: parts sorting before 020 010 only part 010: exit 0 sites / exit 66 / markers 0 / 2 / 2 part 010: check_skip calls 0 part 430's arms: PGC_EXIT_SKIPPED=66 in lib.sh 1 exit-0 and bare-exit sweep over all parts 0 driver handed a bad pg_config rc=2, no summary accounting assertions still on the 4-field form none driver-free pytest job 10 files, 183 passed Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EbyGSaU93XYQr8aH4NrUiw
…ew rules (#918) Merging the updated #923 base, which now carries #939, #940 and #941, puts this branch under two rules that did not exist when it was written. Neither is a conflict git could have shown me: both are arms that read the WHOLE tree, so a file this branch never touched changed what this branch has to satisfy. THE MANIFEST, which is this PR's own arm firing correctly. Part 420 compares `parts.manifest` against the glob in both directions. #940 and #941 added parts 430 and 440 to main, so the manifest was two short and the arm would have gone red -- which is the guard working rather than a merge problem. @OffgridwithJD called this exact ordering on both of their PRs before either landed. on disk 44, listed 42, only-on-disk: 430-..., 440-... after, on disk 44, listed 44, both directions empty still live: drop 430 from the comparison and it is named again Added in sorted position rather than appended, because the file is sorted and a manifest that stops being sorted is a diff nobody can read. AND ONE MORE STRING-COMPARED `grep -c`, MINE, at `run_all_versions.sh:1382`: [ "$(grep -c '^RESULT\t' "$builddir/${s}.log" || true)" != 0 ] Zero such sites on main and zero on the #923 base, so this branch added it -- #929's defect, reintroduced while #941 was closing it. `grep -c` prints nothing on a pattern that does not compile and `[ "" != 0 ]` is TRUE, so this answers "the log has RESULT records" for a question grep never asked. One input to grep, checked, which is #941's condition for the conversion being behaviour-preserving. #941's sweep on the merged tree, before 1 after 0 still live: one planted back 1 Checked on the merged tree, not on this branch alone: #940's exit-0 and bare-exit sweep 0 the driver handed a bad pg_config rc=2, no summary ledger control / BOGUS verdict / one rc=0 / rc=2 / rc=2 --mutation against two failures driver-free pytest job 195 passed Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EbyGSaU93XYQr8aH4NrUiw
raises-catches-setup— entry 5, the last onVACUITY_MODES.md's list of what to add next.The statement count rule refuses a
pytest.raisesblock holding more than one top-level statement. Two shapes are one statement and still perform the setup inside the block, so the count saw nothing:Both were measured against the shipped scan reporting
1 passed, exit 0, zero offences, with the setup raising and the statement under test never running. The two arms that recorded them as residuals now assert the refusal, so the closure is a measurement rather than a sentence.The fix is not a deeper count
Counting recursively would catch both and would also refuse a legitimate single-statement loop — which is why the entry always said the fix is a claim about which statement raised. Two rules:
if,while,withandtrynest exactly asfordoes, and a rule catching onlyforcloses an example rather than a mode. The kinds are looked up by name, becauseTryStarandMatchexist only on newer Pythons and a missing attribute would be aNameErrorat import rather than a rule that quietly does less.The rule turns on where the function is defined, not on the statement being a call, and that is what makes the budget zero.
Measured first, and it changed the design
Before writing either rule: five
pytest.raisesblocks in real code across the corpus, and none touch a database. Four callbuild_and_install, imported from the module under test; one calls a method. The scan reports no offence on any of them.The entry suggests "a helper that runs exactly one statement and owns the assertion" — a DB helper. With zero SQL-raising blocks in the corpus that helper would have had no call sites, an instrument with nothing exercising it, which this directory refuses to build. The shape rules close the same mode against the code that exists.
The residual is a method. A method that performs setup and then the statement is invisible to this rule, and no static rule can see inside it. Stated rather than hidden, and pinned by the arm that accepts the method shape.
Prove by removal
Five mutations, each by exact string with a parse assertion, and the restore verified byte-identical rather than by
git diff:test_raises_sqlstate.pyforcounts as compoundTwo of my own arms could not fail until I mutated them
Exprchanged nothing andx = _helper()stayed open.Foralone was invisible.And the rules fired while printing nothing
The message assembly buckets offenders by substring, and that bucket's filter was the exact sentence of the count rule. So both new rules refused the run and the layer printed
refuses this run: .with an empty list — the arms reddened on a missing message while the refusal itself worked, which is a guard that cannot be told from an unfired one. The filter is now the common tail of all three phrases.Recorded while I was in there
The comment defending the one-source-line offence phrase cited selftest 440 grepping this file and counting the copies. Selftest 440 no longer exists — #927 deleted it under the harness-independence rule. Nothing greps this source for the phrase today, so that form is now a convention rather than a guarded property; what is still load-bearing is the runtime string the pytest arms match, which a split f-string would not change at all. The comment says that instead.
The count is a function of the merge
#935 moved the refused total 27 → 28. This branch moved it 27 → 28 as well. The merge was clean and therefore wrong: git took one 28 where the answer is 29, because the number is a property of both closures rather than of either. That is the failure
TESTS.md's own header records about counts in this document, and the reason they are checked by arms. Every number below is what the arms reported against the ids on disk, not a number I chose:Gate
Merged with main
79882992(#935 landed). Independent of #936, which closes a different mode.🤖 Generated with Claude Code
https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a